feat(flags): Add retry support for feature flag requests #392
+405
−68
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
haacked
force-pushed
the
haacked/retries
branch
2 times, most recently
from
9bb9617 to
4b16ae9
Compare
haacked
changed the title
haacked
force-pushed
the
haacked/retries
branch
from
4b16ae9 to
687465b
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
haacked
force-pushed
the
haacked/retries
branch
2 times, most recently
from
8fb57c9 to
08687d7
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
haacked
force-pushed
the
haacked/retries
branch
from
08687d7 to
28c7e09
Compare
Use urllib3's built-in Retry mechanism for feature flag POST requests instead of application-level retry logic. This is simpler and leverages well-tested library code. Key changes: - Add `RETRY_STATUS_FORCELIST` = [408, 500, 502, 503, 504] - Add `_build_flags_session()` with POST retries and `status_forcelist` - Update `flags()` to use dedicated flags session - Add tests for retry configuration and session usage The flags session retries on: - Network failures (connect/read errors) - Transient server errors (408, 500, 502, 503, 504) It does NOT retry on: - 429 (rate limit) - need to wait, not hammer - 402 (quota limit) - won't resolve with retries
haacked
force-pushed
the
haacked/retries
branch
from
28c7e09 to
eba2bbd
Compare
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
haacked
force-pushed
the
haacked/retries
branch
from
dc5bcad to
6e3ca18
Compare
dustinbyrne
approved these changes
Dec 16, 2025
Contributor
dustinbyrne
left a comment
dustinbyrne
left a comment
There was a problem hiding this comment.
Looks good - one comment about the network failures 👍
Add tests that verify actual retry behavior, not just configuration: - test_retries_on_503_then_succeeds: Spins up a local HTTP server that returns 503 twice then 200, verifying 3 requests are made - test_connection_errors_are_retried: Verifies connection errors trigger retries by measuring elapsed time with backoff Both tests use dynamically allocated ports for CI safety.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
Feature flag requests can fail due to transient issues like read timeouts, connection errors, or temporary server problems. Currently these failures bubble up immediately without any retry attempt.
Example error:
Solution
Create a dedicated HTTP session for feature flag requests configured with urllib3's
Retry:This leverages urllib3's battle-tested retry logic rather than adding application-level retry code.
Why not retry 429?
Rate limits indicate the client should back off, not immediately retry. Retrying would make the problem worse.
Related
enable_keep_alive()) which addresses idle connection dropsExamples
I also updated example.py so it can run without a personal api key set. This allows testing the sdk against /flags.